Skip to content

Remove nimOldCaseObjects & Bump nim-confutils#7669

Merged
tersec merged 3 commits into
unstablefrom
remove_oldcaseobject
Oct 31, 2025
Merged

Remove nimOldCaseObjects & Bump nim-confutils#7669
tersec merged 3 commits into
unstablefrom
remove_oldcaseobject

Conversation

@nitely

@nitely nitely commented Oct 27, 2025

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Oct 27, 2025

Copy link
Copy Markdown

Unit Test Results

       15 files  ±0    3 030 suites  ±0   1h 45m 33s ⏱️ + 13m 41s
12 625 tests ±0  12 045 ✔️ ±0  580 💤 ±0  0 ±0 
79 371 runs  ±0  78 494 ✔️ ±0  877 💤 ±0  0 ±0 

Results for commit 70491a4. ± Comparison against base commit 4b024bd.

♻️ This comment has been updated with latest results.

@nitely
nitely marked this pull request as ready for review October 27, 2025 17:32
@nitely
nitely requested a review from tersec October 27, 2025 17:32
@tersec

tersec commented Oct 30, 2025

Copy link
Copy Markdown
Contributor

Well, some of the serialization also uses Nim old case objects, I believe.

@nitely
nitely marked this pull request as draft October 30, 2025 16:04
@nitely
nitely marked this pull request as ready for review October 30, 2025 17:56
@nitely

nitely commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

@tersec I just added switch("warningAsError", "CaseTransition:on") to config to make sure the compilation fails if that's the case. nim-serialization does not support case objects (yet). Json and toml serializers fail with the nim-ser error, I checked.

Only nim-json-serilizer has it defined but for a test that overloads a readValue case object.

Unless there is some case where the Nim compiler does not warn about CaseTransition and the nimbus test suite does not cover that case, it should be fine. Otherwise, it'll throw a runtime FieldDefect.

@tersec
tersec merged commit 02e3610 into unstable Oct 31, 2025
12 checks passed
@tersec
tersec deleted the remove_oldcaseobject branch October 31, 2025 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants